-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow manual initialization and config as code #1149
Conversation
Deploy preview for netlify-cms-www ready! Built with commit c5feb8b |
Deploy preview for cms-demo ready! Built with commit c5feb8b |
I'll update the config spec to get tests passing in the am. |
@@ -41,7 +41,6 @@ | |||
"setupFiles": [ | |||
"./setupTests.js" | |||
], | |||
"mapCoverage": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer required in Jest 22.
"jest": "^21.2.1", | ||
"jest-cli": "^21.2.1", | ||
"jest": "^22.0.0", | ||
"jest-cli": "^22.0.0", | ||
"lint-staged": "^3.3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jest updated for fix of toEqual
requiring matching order when comparing maps.
@@ -1,117 +1,120 @@ | |||
import { fromJS } from 'immutable'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Causes for change in this file:
- Config objects are now converted to immutable maps before processing, so functions like
applyDefaults
now expect a map. - The
isFetching
config state value is nowtrue
by default.
import { authenticateUser } from "Actions/auth"; | ||
import * as publishModes from "Constants/publishModes"; | ||
|
||
export const CONFIG_REQUEST = "CONFIG_REQUEST"; | ||
export const CONFIG_SUCCESS = "CONFIG_SUCCESS"; | ||
export const CONFIG_FAILURE = "CONFIG_FAILURE"; | ||
export const CONFIG_MERGE = "CONFIG_MERGE"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows configuration to be merged into the state without triggering CONFIG_SUCCESS
, which should only occur after an intentional configuration retrieval cycle.
@@ -1,50 +1,63 @@ | |||
import yaml from "js-yaml"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Causes for change in this file:
- Config objects are now converted to immutable maps before processing.
- We must now differentiate between merging a config value into state and loading config state into the application, since state values could be merged in an arbitrary number of times before the final configuration state is ready.
@@ -0,0 +1,67 @@ | |||
import React from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is simply a paste of the core application initialization code that used to be in index.js
. It was created to be shared between index.js
for auto init, and init.js
for manual init.
@@ -1,54 +1,16 @@ | |||
import React from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to this file are limited to moving the core initialization out to bootstrap.js
.
@@ -0,0 +1,7 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file will be distributed in our npm module, and provides functions for retrieving the manual initialization function and registry.
@@ -1,4 +1,4 @@ | |||
import { OrderedMap, fromJS } from 'immutable'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Causes for change in this file:
action.payload
is now an immutable map instead of an object.- Logic needed a little cleanup.
@@ -1,14 +1,21 @@ | |||
import Immutable from 'immutable'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Causes for change in this file:
action.payload
is an immutable map instead of an object.- isFetching is set to
true
by default. - State can be merged prior to configuration finalization.
Cool idea. Docs LGTM. 👍 |
- Summary
This pull request solves at least two problems:
These are both relevant concerns for customizing Netlify CMS for specific static site generators, where we could potentially infer an entire configuration based on SSG conventions, and have no easy way to inject the configuration.
- Description for the changelog
allow manual initialization and config injection
- Notes for reviewers
There are PR comments explaining why changes took place for most individual files. Here are the high points:
Config action payloads are now Immutable maps
Config action payloads used to be objects, and conversion took place in the reducer. This sort of worked, but we now need to load in pieces of configuration one or more times before fetching
config.yml
, and then run validation and whatnot, so now we convert to Immutable right away before doing anything else. A large portion of changes are due to this.Default config state is set to
{ isFetching: true }
The app used to first check for a non-null config state, and then for
isFetching
to be false, before loading up. Now that config can be injected during bootstrapping, these are both false positives. SettingisFetching
to true by default lets us avoid a lot of changes in how we handle configuration state.A secondary entry point
We'll now be distributing
init.js
, which exportsinit
andregistry
functions. Theinit
function accepts an optional object, which is currently only expected to contain an object at theconfig
key. The registry function can be used before or after initialization.Public beta features
Finally, this PR introduces an approach for public betas. We simply release the beta feature and document it in the Beta Features section of the docs, which carries a clear disclaimer. The new docs page is added with this PR, here's the preview: https://deploy-preview-1149--netlify-cms-www.netlify.com/docs/beta-features/